Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Costmap_2d Static & Inflation plugin layer port & Removing Boost dependencies #129

Merged
merged 4 commits into from
Oct 3, 2018

Conversation

bpwilcox
Copy link

@bpwilcox bpwilcox commented Oct 2, 2018

In this PR, the static layer & inflation layer plugins have been ported and compile properly. After a manual inspection of the plugins, they appear to work fine and generate reasonable results, though some additional work needs to be done in order to incorporate the plugins properly in the Costmap2DROS class (particularly with parameters).

Some intial work on the observation buffer was done but was blocked from completion until tf2_sensor_msgs is also ported (will file an issue).

Also, Boost dependencies have been removed from costmap_2d.

This was referenced Oct 2, 2018
@@ -42,6 +42,7 @@
<depend>xmlrpcpp</depend>
<depend>pluginlib</depend>
<depend>libpcl-all-dev</depend>
<depend>message_filters</depend>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing is off

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why my formatting keeps getting off on these files (package.xml & CMakeLists.txt), I have to keep fixing this. Does opening it in gedit somehow autofix the formatting?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't think so, but I'd recommend a 'real' editor as you main choice anyhow. Then you can configure it to do the right thing.

ros::NodeHandle nh("~/" + name_), g_nh;
auto nh = rclcpp::Node::make_shared(name_);
rclcpp::Node::SharedPtr g_nh;
auto parameters_client = std::make_shared<rclcpp::SyncParametersClient>(nh);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use the parameter_client built into the node.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made an issue for this in #119. I was planning to make this replacement all in one PR.

@@ -59,9 +61,13 @@ unsigned int countValues(costmap_2d::Costmap2D & costmap, unsigned char value, b

void addStaticLayer(costmap_2d::LayeredCostmap & layers, tf2_ros::Buffer & tf)
{

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for all these blank lines.

@@ -62,72 +63,68 @@ StaticLayer::~StaticLayer()

void StaticLayer::onInitialize()
{

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete blank lines

add_library(layers
plugins/inflation_layer.cpp

# TODO(bpwilcox): port additional plugin layers
#plugins/static_layer.cpp
# TODO(bpwilcox): port additional plugin layers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing is off

Copy link

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok with me once you address the items that Carl pointed out.

more spacing fixes

one more line fix
@bpwilcox bpwilcox merged commit 63c5b35 into ros-navigation:master Oct 3, 2018
ghost pushed a commit to logivations/navigation2 that referenced this pull request Mar 7, 2022
* [AMRFM-958] Renamed agv_gazebo -> amr_gazebo

* [AMRFM-958] Renamed AgvError and AssignBinToAgv

* gazebo entity rename

Co-authored-by: HovorunB <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>
Co-authored-by: Tony Najjar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants